fix: Refresh ~/.finch/confg.json based on finch.yaml #966

haytok · 2024-06-05T14:16:59Z

Suppose we have configured the creds_helpers in ~/.finch/finch.yaml as follows, and subsequently initialized a VM (finch vm init).

cpus: 6
creds_helpers:
    - ecr-login
memory: 8GiB
vmType: vz
rosetta: true

As a result, ~/.finch/config.json is created, and it contains the following:

{"credsStore":"ecr-login"}

This allows us to utilize the Amazon ECR Docker Credential Helper within Finch.

Subsequently, suppose we stop and remove the VM
(finch vm stop && finch vm remove), and then remove the creds_helpers configuration from finch.yaml.

We then configure the finch.yaml file as follows:

cpus: 6
memory: 8GiB
vmType: vz
rosetta: true

As a result, when we reinitialize the VM (finch vm init), the expected behavior is that it will no longer use the Amazon ECR Docker Credential Helper.

However, when initializing the VM, despite the absence of creds_helpers configuration in finch.yaml, the "credsStore": "ecr-login" remains in config.json, allowing the continued use of the Amazon ECR Docker Credential Helper.

This behavior has been reported in the following issue:

Still authenticated after removing creds_helpers from Finch VM configuration #480

Furthermore, this issue occurs when we stop the VM (finch vm stop), modify finch.yaml, and subsequently start the VM (finch vm start).

Consequently, we will modify the behavior to update config.json in accordance with the creds_helpers configuration in finch.yaml when initiating or starting the VM.

Issue #, if available: #480

Description of changes: The details are described in the commit message.

Testing done: yes

I've reviewed the guidance in CONTRIBUTING.md

License Acceptance

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

haytok · 2024-06-05T16:47:45Z

The Unit Test is failing in the Windows environment.

 -test.shuffle 1717597156217190000
--- FAIL: Test_RefreshConfigFile (0.00s)
    --- FAIL: Test_RefreshConfigFile/should_return_an_error_if_finch.yaml_is_invalid (0.00s)
        cred_helper_test.go:166: 
            	Error Trace:	D:/a/finch/finch/pkg/dependency/credhelper/cred_helper_test.go:166
            	Error:      	Not equal: 
            	            	expected: true
            	            	actual  : false
            	Test:       	Test_RefreshConfigFile/should_return_an_error_if_finch.yaml_is_invalid
FAIL
FAIL	github.com/runfinch/finch/pkg/dependency/credhelper	0.024s

This is occurring because in the Windows environment, the validate() function is not accurately validating the contents of finch.yam.

https://github.com/runfinch/finch/blob/main/pkg/config/validate_windows.go#L13C1-L15C2

As a result, we will modify the code to separate the Unit Test for macOS (darwin) and Windows environments.

haytok · 2024-06-06T13:01:05Z

The following CI is failing, and I'm currently investigating the cause of the failure as it remains unclear.

https://github.com/runfinch/finch/actions/runs/9388468523/job/25868854684?pr=966

The details of the error are as follows:

Details

panic: test timed out after 2h0m0s
running tests:
	TestVM (2h0m0s)

goroutine 82 [running]:
testing.(*M).startAlarm.func1()
	C:/Program Files/Go/src/testing/testing.go:2366 +0x385
created by time.goFunc
	C:/Program Files/Go/src/time/sleep.go:177 +0x2d

goroutine 1 [chan receive, 119 minutes]:
testing.(*T).Run(0xc00017a9c0, {0xeba887?, 0xc03877a3c0?}, 0xf04c70)
	C:/Program Files/Go/src/testing/testing.go:1750 +0x3ab
testing.runTests.func1(0xc00017a9c0)
	C:/Program Files/Go/src/testing/testing.go:2161 +0x37
testing.tRunner(0xc00017a9c0, 0xc0000f1c70)
	C:/Program Files/Go/src/testing/testing.go:1689 +0xfb
testing.runTests(0xc000178228, {0x138eb40, 0x1, 0x1}, {0x1?, 0x9dd493?, 0x13c5680?})
	C:/Program Files/Go/src/testing/testing.go:2159 +0x445
testing.(*M).Run(0xc00011fe00)
	C:/Program Files/Go/src/testing/testing.go:2027 +0x68b
main.main()
	_testmain.go:47 +0x16c

goroutine 18 [runnable]:
github.com/onsi/ginkgo/v2/internal.extractRunningGoroutines()
	C:/Users/Administrator/go/pkg/mod/github.com/onsi/ginkgo/[email protected]/internal/progress_report.go:177 +0xef3
github.com/onsi/ginkgo/v2/internal.NewProgressReport(_, {{0xc0000be3c0, 0x2, 0x2}, {0xc00018c1c0, 0x2, 0x2}, {0xc0000305d0, 0x2, 0x2}, ...}, ...)
	C:/Users/Administrator/go/pkg/mod/github.com/onsi/ginkgo/[email protected]/internal/progress_report.go:75 +0x2df
github.com/onsi/ginkgo/v2/internal.(*Suite).generateProgressReport(_, _)
	C:/Users/Administrator/go/pkg/mod/github.com/onsi/ginkgo/[email protected]/internal/suite.go:381 +0x59b
github.com/onsi/ginkgo/v2/internal.(*Suite).runNode(_, {0x16, 0x4, {0xedaf82, 0x33}, 0xc000113710, {{0x105e722, 0x42}, 0x19, {0x0, ...}, ...}, ...}, ...)
	C:/Users/Administrator/go/pkg/mod/github.com/onsi/ginkgo/[email protected]/internal/suite.go:959 +0x119f
github.com/onsi/ginkgo/v2/internal.(*group).attemptSpec(0xc00038f130, 0x1, {{0xc0002d1c08?, 0xc00018c1c0?, 0x2?}, 0x0?})
	C:/Users/Administrator/go/pkg/mod/github.com/onsi/ginkgo/[email protected]/internal/group.go:199 +0xc33
github.com/onsi/ginkgo/v2/internal.(*group).run(0xc00038f130, {0xc0000be000?, 0x0?, 0x0?})
	C:/Users/Administrator/go/pkg/mod/github.com/onsi/ginkgo/[email protected]/internal/group.go:349 +0x925
github.com/onsi/ginkgo/v2/internal.(*Suite).runSpecs(0xc00014ea88, {0xecd48a, 0x1f}, {0x144df20, 0x0, 0x0}, {0xc00016edb0, 0x2a}, 0x0, {0xc00013d688, ...})
	C:/Users/Administrator/go/pkg/mod/github.com/onsi/ginkgo/[email protected]/internal/suite.go:489 +0xac5
github.com/onsi/ginkgo/v2/internal.(*Suite).Run(_, {_, _}, {_, _, _}, {_, _}, _, {0xfaf790, ...}, ...)
	C:/Users/Administrator/go/pkg/mod/github.com/onsi/ginkgo/[email protected]/internal/suite.go:130 +0x405
github.com/onsi/ginkgo/v2.RunSpecs({0xfa78c0, 0xc00017ab60}, {0xecd48a, 0x1f}, {0x0, 0x0, 0x0})
	C:/Users/Administrator/go/pkg/mod/github.com/onsi/ginkgo/[email protected]/core_dsl.go:300 +0x8b3
github.com/runfinch/finch/e2e/vm.TestVM(0xc00017ab60)
	C:/actions-runner/_work/finch/finch/e2e/vm/vm_windows_test.go:55 +0x1ca
testing.tRunner(0xc00017ab60, 0xf04c70)
	C:/Program Files/Go/src/testing/testing.go:1689 +0xfb
created by testing.(*T).Run in goroutine 1
	C:/Program Files/Go/src/testing/testing.go:1742 +0x390

goroutine 19 [syscall, 119 minutes]:
os/signal.signal_recv()
	C:/Program Files/Go/src/runtime/sigqueue.go:152 +0x29
os/signal.loop()
	C:/Program Files/Go/src/os/signal/signal_unix.go:23 +0x13
created by os/signal.Notify.func1.1 in goroutine 18
	C:/Program Files/Go/src/os/signal/signal.go:151 +0x1f

goroutine 20 [select, 119 minutes]:
github.com/onsi/ginkgo/v2/internal/interrupt_handler.(*InterruptHandler).registerForInterrupts.func2(0x0?)
	C:/Users/Administrator/go/pkg/mod/github.com/onsi/ginkgo/[email protected]/internal/interrupt_handler/interrupt_handler.go:131 +0x7d
created by github.com/onsi/ginkgo/v2/internal/interrupt_handler.(*InterruptHandler).registerForInterrupts in goroutine 18
	C:/Users/Administrator/go/pkg/mod/github.com/onsi/ginkgo/[email protected]/internal/interrupt_handler/interrupt_handler.go:128 +0x165

goroutine 21 [select, 119 minutes]:
github.com/onsi/ginkgo/v2/internal.RegisterForProgressSignal.func1()
	C:/Users/Administrator/go/pkg/mod/github.com/onsi/ginkgo/[email protected]/internal/progress_report.go:32 +0x8b
created by github.com/onsi/ginkgo/v2/internal.RegisterForProgressSignal in goroutine 18
	C:/Users/Administrator/go/pkg/mod/github.com/onsi/ginkgo/[email protected]/internal/progress_report.go:30 +0xd9

goroutine 26 [syscall, 119 minutes, locked to thread]:
syscall.SyscallN(0x7ffa06a23bf0?, {0xc0004b1a48?, 0x3?, 0x0?})
	C:/Program Files/Go/src/runtime/syscall_windows.go:544 +0x107
syscall.Syscall(0x3?, 0x3?, 0xc00049e900?, 0x0?, 0x0?)
	C:/Program Files/Go/src/runtime/syscall_windows.go:482 +0x35
syscall.WaitForSingleObject(0x30c, 0xffffffff)
	C:/Program Files/Go/src/syscall/zsyscall_windows.go:1142 +0x5d
os.(*Process).wait(0xc0003356b0)
	C:/Program Files/Go/src/os/exec_windows.go:18 +0x50
os.(*Process).Wait(...)
	C:/Program Files/Go/src/os/exec.go:134
os/exec.(*Cmd).Wait(0xc000160160)
	C:/Program Files/Go/src/os/exec/exec.go:897 +0x45
os/exec.(*Cmd).Run(0xc000160160)
	C:/Program Files/Go/src/os/exec/exec.go:607 +0x2d
github.com/runfinch/finch/e2e/vm.init.func5(0xc0002981b0)
	C:/actions-runner/_work/finch/finch/e2e/vm/vm_test.go:32 +0x2f9
github.com/runfinch/finch/e2e/vm.init.func7.1.1()
	C:/actions-runner/_work/finch/finch/e2e/vm/additional_disk_test.go:26 +0x3a
github.com/onsi/ginkgo/v2/internal.extractBodyFunction.func3({0x0?, 0x0?})
	C:/Users/Administrator/go/pkg/mod/github.com/onsi/ginkgo/[email protected]/internal/node.go:472 +0x13
github.com/onsi/ginkgo/v2/internal.(*Suite).runNode.func3()
	C:/Users/Administrator/go/pkg/mod/github.com/onsi/ginkgo/[email protected]/internal/suite.go:894 +0x8d
created by github.com/onsi/ginkgo/v2/internal.(*Suite).runNode in goroutine 18
	C:/Users/Administrator/go/pkg/mod/github.com/onsi/ginkgo/[email protected]/internal/suite.go:881 +0xdb0
FAIL	github.com/runfinch/finch/e2e/vm	7200.074s
FAIL
make: *** [test-e2e-vm-serial] Error 1
Error: Process completed with exit code 1.

Additionally, the CI for recently created PR is also failing.

https://github.com/runfinch/finch/actions/runs/9392175073/job/25865982300?pr=969

Consequently, it is reasonable to assume that the changes made in this commit are not the root cause of the failure.

Suppose we have configured the `creds_helpers` in `~/.finch/finch.yaml` as follows, and subsequently initialized a VM (`finch vm init`). ``` cpus: 6 creds_helpers: - ecr-login memory: 8GiB vmType: vz rosetta: true ``` As a result, `~/.finch/config.json` is created, and it contains the following: ``` {"credsStore":"ecr-login"} ``` This allows us to utilize the Amazon ECR Docker Credential Helper within Finch. Subsequently, suppose we stop and remove the VM (`finch vm stop` && `finch vm remove`), and then remove the `creds_helpers` configuration from `finch.yaml`. We then configure the `finch.yaml` file as follows: ``` cpus: 6 memory: 8GiB vmType: vz rosetta: true ``` As a result, when we reinitialize the VM (`finch vm init`), the expected behavior is that it will no longer use the Amazon ECR Docker Credential Helper. However, when initializing the VM, despite the absence of `creds_helpers` configuration in `finch.yaml`, the `"credsStore": "ecr-login"` remains in `config.json`, allowing the continued use of the Amazon ECR Docker Credential Helper. This behavior has been reported in the following issue: - runfinch#480 Furthermore, this issue occurs when we stop the VM (`finch vm stop`), modify `finch.yaml`, and subsequently start the VM (`finch vm start`). Consequently, we will modify the behavior to update `config.json` in accordance with the `creds_helpers` configuration in `finch.yaml` when initiating or starting the VM. Note that in this pull request, the commits are divided as follows: - Implement the logic to update `config.json` according to `finch.yaml` and change the function name (loadFinchConfig) in the config package - Modify to call the logic to update `config.json` according to `finch.yaml` on the VM side - Add unit tests for the credhelper package and modify unit tests for the config package - Modify and add Behavior-Driven Development (BDD) Tests using Ginkgo on the VM side - Add e2e tests On the other hand, in this commit, the logic to update `config.json` according to `finch.yaml` is implemented and the function name (loadFinchConfig) is changed in the config package. Signed-off-by: Hayato Kiwata <[email protected]>

This commit fixes the logic to call the VM side to update config.json according to ~/.finch/finch.yaml. Signed-off-by: Hayato Kiwata <[email protected]>

…kage In this commit, unit tests for credhelper package are added and unit tests for config package are modified. Signed-off-by: Hayato Kiwata <[email protected]>

In this commit, Behavior-Driven Development (BDD) tests using Ginkgo are modified and added. Signed-off-by: Hayato Kiwata <[email protected]>

In this commit, e2e tests for the implementation are added. Signed-off-by: Hayato Kiwata <[email protected]>

haytok · 2024-06-11T01:20:56Z

The following error has occured.

https://github.com/runfinch/finch/actions/runs/9428915702/job/26026006047?pr=966

Details

Finch Container Development E2E Tests Run a container image when running a container with network related flags should add a custom host-to-IP mapping with --add-host flag with special IP
C:/Users/Administrator/go/pkg/mod/github.com/runfinch/[email protected]/tests/run.go:383
  time="2024-06-10T14:27:57Z" level=debug msg="Creating limactl command: ARGUMENTS: [shell --workdir /mnt/c/actions-runner/_work/finch/finch/e2e/container finch sudo -E nerdctl ps --all --quiet --no-trunc], LIMA_HOME: C:\\actions-runner\\_work\\finch\\finch\\_output\\lima\\data"
  6639458223931720f48de6b6afdc3b33dfb632bfc7fd55c56488d89b3fc6638d
  No containers to be removed
  time="2024-06-10T14:27:58Z" level=debug msg="Creating limactl command: ARGUMENTS: [shell --workdir /mnt/c/actions-runner/_work/finch/finch/e2e/container finch sudo -E nerdctl images --all --quiet], LIMA_HOME: C:\\actions-runner\\_work\\finch\\finch\\_output\\lima\\data"
  4fac7a8257b1
  No images to be removed
  time="2024-06-10T14:27:59Z" level=debug msg="Creating limactl command: ARGUMENTS: [shell --workdir /mnt/c/actions-runner/_work/finch/finch/e2e/container finch sudo -E nerdctl volume ls --quiet], LIMA_HOME: C:\\actions-runner\\_work\\finch\\finch\\_output\\lima\\data"
  fbd4a280b3ac12a87be6b4c9d4c1381023134854d65b67676f90f96c27a6a30f
  time="2024-06-10T14:27:59Z" level=debug msg="Creating limactl command: ARGUMENTS: [shell --workdir /mnt/c/actions-runner/_work/finch/finch/e2e/container finch sudo -E nerdctl volume prune --force --all], LIMA_HOME: C:\\actions-runner\\_work\\finch\\finch\\_output\\lima\\data"
  time="2024-06-10T14:28:00Z" level=debug msg="Creating limactl command: ARGUMENTS: [shell --workdir /mnt/c/actions-runner/_work/finch/finch/e2e/container finch sudo -E nerdctl network ls --format {{.Name}}], LIMA_HOME: C:\\actions-runner\\_work\\finch\\finch\\_output\\lima\\data"
  bridge
  host
  none
  No networks to be removed
  time="2024-06-10T14:28:06Z" level=debug msg="Resolving special IP \"host-gateway\" to \"172.20.32.1\" for host \"test-host\""
  time="2024-06-10T14:28:06Z" level=debug msg="Creating limactl command: ARGUMENTS: [shell --workdir /mnt/c/actions-runner/_work/finch/finch/e2e/container finch sudo -E nerdctl container run -d --name ctr-test --add-host test-host:172.20.32.1 localhost:54245/amazonlinux/amazonlinux:2 sleep infinity], LIMA_HOME: C:\\actions-runner\\_work\\finch\\finch\\_output\\lima\\data"
  2024-06-10 14:28:07.000679828 +0000 UTC finch /snapshot/prepare {"key":"extract-980825813-ceyw sha256:9e1c1e301fa54d41bb458ac8eb55c27666b79d815b1955e62d45edb27a814e3c","snapshotter":"overlayfs"}
  localhost:54245/amazonlinux/amazonlinux:2:                                        resolved       |++++++++++++++++++++++++++++++++++++++| 
  index-sha256:22bf06725070287a989d60f6ad5c036957515956c0408c4140839a18a632ce88:    done           |++++++++++++++++++++++++++++++++++++++| 
  manifest-sha256:d50f1e912a2da0036b3e2303c2dea1788579e41cdfb0643e94ea354e92bdf132: done           |++++++++++++++++++++++++++++++++++++++| 
  config-sha256:24c69f972b4922d2e8c98dd360dc8717646a27703f7fa80f8ca820937fd0bd48:   downloading    |--------------------------------------|    0.0 B/1.4 KiB 
  elapsed: 0.1 s                                                                    total:  846.0  (8.1 KiB/s)                                       
  localhost:54245/amazonlinux/amazonlinux:2:                                        resolved       |++++++++++++++++++++++++++++++++++++++| 
  index-sha256:22bf06725070287a989d60f6ad5c036957515956c0408c4140839a18a632ce88:    done           |++++++++++++++++++++++++++++++++++++++| 
  manifest-sha256:d50f1e912a2da0036b3e2303c2dea1788579e41cdfb0643e94ea354e92bdf132: done           |++++++++++++++++++++++++++++++++++++++| 
  config-sha256:24c69f972b4922d2e8c98dd360dc8717646a27703f7fa80f8ca820937fd0bd48:   done           |++++++++++++++++++++++++++++++++++++++| 
  layer-sha256:8a0995d2bf38a3ce742d376865782dab667a8fd7d3ded687b5775794788440be:    downloading    |+++-----------------------------------|  5.0 MiB/59.7 MiB 
  elapsed: 0.2 s                                                                    total:  5.0 Mi (24.8 MiB/s)                                      
  localhost:54245/amazonlinux/amazonlinux:2:                                        resolved       |++++++++++++++++++++++++++++++++++++++| 
  index-sha256:22bf06725070287a989d60f6ad5c036957515956c0408c4140839a18a632ce88:    done           |++++++++++++++++++++++++++++++++++++++| 
  manifest-sha256:d50f1e912a2da0036b3e2303c2dea1788579e41cdfb0643e94ea354e92bdf132: done           |++++++++++++++++++++++++++++++++++++++| 
  config-sha256:24c69f972b4922d2e8c98dd360dc8717646a27703f7fa80f8ca820937fd0bd48:   done           |++++++++++++++++++++++++++++++++++++++| 
  layer-sha256:8a0995d2bf38a3ce742d376865782dab667a8fd7d3ded687b5775794788440be:    downloading    |+++++++++++++++++---------------------| 27.0 MiB/59.7 MiB 
  elapsed: 0.3 s                                                                    total:  27.0 M (89.3 MiB/s)                                      
  localhost:54245/amazonlinux/amazonlinux:2:                                        resolved       |++++++++++++++++++++++++++++++++++++++| 
  index-sha256:22bf06725070287a989d60f6ad5c036957515956c0408c4140839a18a632ce88:    done           |++++++++++++++++++++++++++++++++++++++| 
  manifest-sha256:d50f1e912a2da0036b3e2303c2dea1788579e41cdfb0643e94ea354e92bdf132: done           |++++++++++++++++++++++++++++++++++++++| 
  config-sha256:24c69f972b4922d2e8c98dd360dc8717646a27703f7fa80f8ca820937fd0bd48:   done           |++++++++++++++++++++++++++++++++++++++| 
  layer-sha256:8a0995d2bf38a3ce742d376865782dab667a8fd7d3ded687b5775794788440be:    downloading    |++++++++++++++++++++++++++++++--------| 48.0 MiB/59.7 MiB 
  elapsed: 0.4 s                                                                    total:  48.0 M (119.6 MiB/s)                                     
  localhost:54245/amazonlinux/amazonlinux:2:                                        resolved       |++++++++++++++++++++++++++++++++++++++| 
  index-sha256:22bf06725070287a989d60f6ad5c036957515956c0408c4140839a18a632ce88:    done           |++++++++++++++++++++++++++++++++++++++| 
  manifest-sha256:d50f1e912a2da0036b3e2303c2dea1788579e41cdfb0643e94ea354e92bdf132: done           |++++++++++++++++++++++++++++++++++++++| 
  config-sha256:24c69f972b4922d2e8c98dd360dc8717646a27703f7fa80f8ca820937fd0bd48:   done           |++++++++++++++++++++++++++++++++++++++| 
  layer-sha256:8a0995d2bf38a3ce742d376865782dab667a8fd7d3ded687b5775794788440be:    downloading    |++++++++++++++++++++++++++++++++++++++| 59.7 MiB/59.7 MiB 
  elapsed: 0.5 s                                                                    total:  59.7 M (119.2 MiB/s)                                     
  localhost:54245/amazonlinux/amazonlinux:2:                                        resolved       |++++++++++++++++++++++++++++++++++++++| 
  index-sha256:22bf06725070287a989d60f6ad5c036957515956c0408c4140839a18a632ce88:    done           |++++++++++++++++++++++++++++++++++++++| 
  manifest-sha256:d50f1e912a2da0036b3e2303c2dea1788579e41cdfb0643e94ea354e92bdf132: done           |++++++++++++++++++++++++++++++++++++++| 
  config-sha256:24c69f972b4922d2e8c98dd360dc8717646a27703f7fa80f8ca820937fd0bd48:   done           |++++++++++++++++++++++++++++++++++++++| 
  layer-sha256:8a0995d2bf38a3ce742d376865782dab667a8fd7d3ded687b5775794788440be:    downloading    |++++++++++++++++++++++++++++++++++++++| 59.7 MiB/59.7 MiB 
  elapsed: 0.6 s                                                                    total:  59.7 M (99.3 MiB/s)                                      
  localhost:54245/amazonlinux/amazonlinux:2:                                        resolved       |++++++++++++++++++++++++++++++++++++++| 
  index-sha256:22bf06725070287a989d60f6ad5c036957515956c0408c4140839a18a632ce88:    done           |++++++++++++++++++++++++++++++++++++++| 
  manifest-sha256:d50f1e912a2da0036b3e2303c2dea1788579e41cdfb0643e94ea354e92bdf132: done           |++++++++++++++++++++++++++++++++++++++| 
  config-sha256:24c69f972b4922d2e8c98dd360dc8717646a27703f7fa80f8ca820937fd0bd48:   done           |++++++++++++++++++++++++++++++++++++++| 
  layer-sha256:8a0995d2bf38a3ce742d376865782dab667a8fd7d3ded687b5775794788440be:    done           |++++++++++++++++++++++++++++++++++++++| 
  elapsed: 0.7 s                                                                    total:  59.7 M (85.2 MiB/s)                                      
  localhost:54245/amazonlinux/amazonlinux:2:                                        resolved       |++++++++++++++++++++++++++++++++++++++| 
  22bf06725070
  4fac7a8257b1
  time="2024-06-10T14:28:24Z" level=debug msg="Creating limactl command: ARGUMENTS: [shell --workdir /mnt/c/actions-runner/_work/finch/finch/e2e/container finch sudo -E nerdctl rmi --force 22bf06725070], LIMA_HOME: C:\\actions-runner\\_work\\finch\\finch\\_output\\lima\\data"
  2024-06-10 14:28:24.713404676 +0000 UTC finch /images/delete {"name":"localhost:54245/amazonlinux/amazonlinux:2"}
  2024-06-10 14:28:24.71898673 +0000 UTC finch /snapshot/remove {"key":"sha256:9e1c1e301fa54d41bb458ac8eb55c27666b79d815b1955e62d45edb27a814e3c","snapshotter":"overlayfs"}
  Untagged: localhost:54245/amazonlinux/amazonlinux:2@sha256:22bf06725070287a989d60f6ad5c036957515956c0408c4140839a18a632ce88
  Deleted: sha256:9e1c1e301fa54d41bb458ac8eb55c27666b79d815b1955e62d45edb27a814e3c
  time="2024-06-10T14:28:25Z" level=debug msg="Creating limactl command: ARGUMENTS: [shell --workdir /mnt/c/actions-runner/_work/finch/finch/e2e/container finch sudo -E nerdctl images --all --format {{.Repository}}:{{.Tag}}], LIMA_HOME: C:\\actions-runner\\_work\\finch\\finch\\_output\\lima\\data"
  public.ecr.aws/docker/library/registry:latest
  No images to be removed
  time="2024-06-10T14:28:25Z" level=debug msg="Creating limactl command: ARGUMENTS: [shell --workdir /mnt/c/actions-runner/_work/finch/finch/e2e/container finch sudo -E nerdctl volume ls --quiet], LIMA_HOME: C:\\actions-runner\\_work\\finch\\finch\\_output\\lima\\data"
  fbd4a280b3ac12a87be6b4c9d4c1381023134854d65b67676f90f96c27a6a30f
  time="2024-06-10T14:28:26Z" level=debug msg="Creating limactl command: ARGUMENTS: [shell --workdir /mnt/c/actions-runner/_work/finch/finch/e2e/container finch sudo -E nerdctl volume prune --force --all], LIMA_HOME: C:\\actions-runner\\_work\\finch\\finch\\_output\\lima\\data"
  time="2024-06-10T14:28:27Z" level=debug msg="Creating limactl command: ARGUMENTS: [shell --workdir /mnt/c/actions-runner/_work/finch/finch/e2e/container finch sudo -E nerdctl network ls --format {{.Name}}], LIMA_HOME: C:\\actions-runner\\_work\\finch\\finch\\_output\\lima\\data"
  bridge
  host
  none
  No networks to be removed
+ [FAILED] [30.300 seconds]
Finch Container Development E2E Tests Run a container image when running a container with network related flags [It] should add a custom host-to-IP mapping with --add-host flag with special IP
C:/Users/Administrator/go/pkg/mod/github.com/runfinch/[email protected]/tests/run.go:383

  [FAILED] Timed out after 10.008s.
  Expected process to exit.  It did not.
  In [It] at: C:/Users/Administrator/go/pkg/mod/github.com/runfinch/[email protected]/command/command.go:115 @ 06/10/24 14:28:20.263

I'm investigating the cause of the test failure.

pendo324 · 2024-06-20T14:04:49Z

The following error has occured.
* https://github.com/runfinch/finch/actions/runs/9428915702/job/26026006047?pr=966
Details

I'm investigating the cause of the test failure.

I think this is unrelated. Right now, the way we enable Windows => WSL2 communication is via a firewall rule. To make this more reliable, it might be best to install/configure mDNS inside the guest VM rootfs and resolve the special host-gateway IP via "$(hostname).local".

Another option is the new mirrored networking mode but that's not available on all versions of Windows which support WSL (and specifically not available on our test runners)

pendo324

Looks good generally, just left a few comments

pendo324 · 2024-06-26T23:35:48Z

pkg/dependency/credhelper/cred_helper.go

+
+// RefreshConfigFile refreshes config.json according to finch.yaml.
+func RefreshConfigFile(fs afero.Fs, logger flog.Logger, finchConfigPath, configJSONPath string) error {
+	systemDeps := system.NewStdLib()


Can you pass these in as dependencies? Makes mocking/testing simpler

I agree.
However, depending on the discussion below, there is no need to call config.LoadFinchConfig().

fix: Refresh ~/.finch/confg.json based on finch.yaml #966 (comment)

So the following call may no longer be necessary.

systemDeps := system.NewStdLib() mem := fmemory.NewMemory()

pendo324 · 2024-06-26T23:37:37Z

pkg/dependency/credhelper/cred_helper.go

+	systemDeps := system.NewStdLib()
+	mem := fmemory.NewMemory()
+
+	finchCfg, err := config.LoadFinchConfig(fs, finchConfigPath, logger, systemDeps, mem)


Same for config, we can pass it as a dependency to the vm commands, and then to this function instead of loading it again

@pendo324

Does this mean that the fc being passed in the following is passed to newVirtualMachineCommand(), which in turn is passed to newStartVMCommand() and newInitVMCommand(), to avoid re-loading the config?

https://github.com/runfinch/finch/blob/main/cmd/finch/virtual_machine.go#L105

pendo324 · 2024-06-26T23:38:33Z

e2e/vm/finch_config_file_test.go

+		var vmType string
+
+		ginkgo.BeforeEach(func() {
+			if runtime.GOOS == "windows" {
+				vmType = "wsl2"
+			} else {
+				vmType = "vz"
+			}
+		})


Is this actually necessary?

Yes, I think it is necessary.

testFinchConfigFile() is called in both darwin and windows tests, but the vmType in finch.yaml is different for those environments.

https://github.com/runfinch/finch/pull/966/files#diff-f93254f4a5296aed6e2d65f03f29dfab361bf9e87a35f9d9e12a0cfa125f7803R91

The following implementation is also helpful.

https://github.com/runfinch/finch/blob/main/e2e/vm/cred_helper_test.go

haytok · 2024-06-27T15:57:58Z

Thanks you for review!!!

I'm checking ....

haytok self-assigned this Jun 5, 2024

haytok added 5 commits June 8, 2024 21:51

fix: Refresh ~/.finch/confg.json based on finch.yaml

71e165d

This commit fixes the logic to call the VM side to update config.json according to ~/.finch/finch.yaml. Signed-off-by: Hayato Kiwata <[email protected]>

test: Add and modify unit tests for credhelper package and config pac…

e4437b3

…kage In this commit, unit tests for credhelper package are added and unit tests for config package are modified. Signed-off-by: Hayato Kiwata <[email protected]>

test: Modify and add BDD tests

4133789

In this commit, Behavior-Driven Development (BDD) tests using Ginkgo are modified and added. Signed-off-by: Hayato Kiwata <[email protected]>

test: Add e2e tests for the implementation

c231fd2

In this commit, e2e tests for the implementation are added. Signed-off-by: Hayato Kiwata <[email protected]>

haytok force-pushed the refresh-config.json-based-on-finch.yaml branch from 392950c to c231fd2 Compare June 8, 2024 12:57

haytok requested a review from a team June 20, 2024 11:10

pendo324 reviewed Jun 26, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: Refresh ~/.finch/confg.json based on finch.yaml #966

fix: Refresh ~/.finch/confg.json based on finch.yaml #966

haytok commented Jun 5, 2024

haytok commented Jun 5, 2024

haytok commented Jun 6, 2024 •

edited

Loading

haytok commented Jun 11, 2024

pendo324 commented Jun 20, 2024

pendo324 left a comment

pendo324 Jun 26, 2024

haytok Jul 2, 2024

pendo324 Jun 26, 2024

haytok Jul 2, 2024

pendo324 Jun 26, 2024

haytok Jul 2, 2024

haytok commented Jun 27, 2024

fix: Refresh ~/.finch/confg.json based on finch.yaml #966

Are you sure you want to change the base?

fix: Refresh ~/.finch/confg.json based on finch.yaml #966

Conversation

haytok commented Jun 5, 2024

License Acceptance

haytok commented Jun 5, 2024

haytok commented Jun 6, 2024 • edited Loading

haytok commented Jun 11, 2024

pendo324 commented Jun 20, 2024

pendo324 left a comment

Choose a reason for hiding this comment

pendo324 Jun 26, 2024

Choose a reason for hiding this comment

haytok Jul 2, 2024

Choose a reason for hiding this comment

pendo324 Jun 26, 2024

Choose a reason for hiding this comment

haytok Jul 2, 2024

Choose a reason for hiding this comment

pendo324 Jun 26, 2024

Choose a reason for hiding this comment

haytok Jul 2, 2024

Choose a reason for hiding this comment

haytok commented Jun 27, 2024

haytok commented Jun 6, 2024 •

edited

Loading